Please address the comments from this code review:

## Overall Comments
- `build_plug_list` now calls `plugins_dir.iterdir()` unconditionally when the path exists; if `plugins_dir` is a file rather than a directory this will raise `NotADirectoryError`, so consider early-returning or skipping iteration when `not plugins_dir.is_dir()` to match the new test’s intent of treating file paths as an empty local set.
- There appear to be two very similar test modules (`tests/unit/test_cli_plugin_utils.py` and `tests/test_cli_plugin_utils.py`) defining the same helpers and first test; consider consolidating them into a single file to avoid duplication and potential divergence.

## Individual Comments

### Comment 1
<location path="astrbot/cli/utils/plugin.py" line_range="168-177" />
<code_context>
+    local_plugin_names = {plugin["name"] for plugin in result}
</code_context>
<issue_to_address>
**issue (bug_risk):** Using a snapshot set for `local_plugin_names` can reintroduce uninstalled plugins multiple times if `online_plugins` contains duplicate names.

Because `local_plugin_names` is computed once before the loop, it never includes plugins added later in the same loop. If `online_plugins` has duplicate names, the second occurrence will still pass `name not in local_plugin_names` and be appended again, reintroducing duplicates that the previous `any(...)` check avoided. To keep the old behavior, either update/rebuild `local_plugin_names` as you append, keep using the `any(...)` check on `result`, or pre‑deduplicate `online_plugins` by name before iterating.
</issue_to_address>

### Comment 2
<location path="tests/unit/test_cli_plugin_utils.py" line_range="53-62" />
<code_context>
+    )
+
+
+def test_build_plug_list_merges_local_and_remote_plugins(monkeypatch, tmp_path):
+    write_metadata(tmp_path / "local-plugin", "local-plugin", "1.0.0")
+    write_metadata(tmp_path / "unpublished-plugin", "unpublished-plugin", "1.0.0")
+    tmp_path.joinpath("ignored-file").write_text("not a plugin", encoding="utf-8")
+
+    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)
+
+    plugins = build_plug_list(tmp_path)
+    plugins_by_name = {plugin["name"]: plugin for plugin in plugins}
+
+    assert plugins_by_name["local-plugin"]["status"] == PluginStatus.NEED_UPDATE
+    assert plugins_by_name["unpublished-plugin"]["status"] == PluginStatus.NOT_PUBLISHED
+    assert plugins_by_name["remote-only"]["status"] == PluginStatus.NOT_INSTALLED
+    assert len(plugins) == 3
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for the case where the local and remote versions are equal or the local version is newer to ensure `NEED_UPDATE` is not set incorrectly.

This test exercises the `NEED_UPDATE` path (local older), `NOT_PUBLISHED`, and `NOT_INSTALLED`. To fully cover the version comparison, please add a test where:

- local version == remote version, and
- ideally, another where local version > remote version.

Both should assert that the installed plugin status does **not** become `NEED_UPDATE`, to protect against regressions in `VersionComparator.compare_version` and the surrounding `NEED_UPDATE` logic.

Suggested implementation:

```python
def test_build_plug_list_merges_local_and_remote_plugins(monkeypatch, tmp_path):
    write_metadata(tmp_path / "local-plugin", "local-plugin", "1.0.0")
    write_metadata(tmp_path / "unpublished-plugin", "unpublished-plugin", "1.0.0")
    tmp_path.joinpath("ignored-file").write_text("not a plugin", encoding="utf-8")

    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)

    plugins = build_plug_list(tmp_path)
    plugins_by_name = {plugin["name"]: plugin for plugin in plugins}

    assert plugins_by_name["local-plugin"]["status"] == PluginStatus.NEED_UPDATE
    assert plugins_by_name["unpublished-plugin"]["status"] == PluginStatus.NOT_PUBLISHED
    assert plugins_by_name["remote-only"]["status"] == PluginStatus.NOT_INSTALLED
    assert len(plugins) == 3


def test_build_plug_list_does_not_mark_equal_version_as_need_update(
    monkeypatch, tmp_path
):
    # NOTE: adjust the version here so it matches the remote version for "local-plugin"
    # returned by FakeClient to ensure this is truly an "equal version" scenario.
    write_metadata(tmp_path / "local-plugin", "local-plugin", "2.0.0")

    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)

    plugins = build_plug_list(tmp_path)
    plugins_by_name = {plugin["name"]: plugin for plugin in plugins}

    # Protect against regressions: equal versions must not be marked as NEED_UPDATE
    assert (
        plugins_by_name["local-plugin"]["status"] != PluginStatus.NEED_UPDATE
    )


def test_build_plug_list_does_not_mark_newer_local_version_as_need_update(
    monkeypatch, tmp_path
):
    # NOTE: set the local version higher than the remote version for "local-plugin"
    # returned by FakeClient to exercise the "local newer" path.
    write_metadata(tmp_path / "local-plugin", "local-plugin", "3.0.0")

    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)

    plugins = build_plug_list(tmp_path)
    plugins_by_name = {plugin["name"]: plugin for plugin in plugins}

    # Protect against regressions: newer local versions must not be marked as NEED_UPDATE
    assert (
        plugins_by_name["local-plugin"]["status"] != PluginStatus.NEED_UPDATE
    )


def test_build_plug_list_treats_file_plugin_path_as_empty_local_set(
    monkeypatch, tmp_path
):
    plugins_file = tmp_path / "plugins"
    plugins_file.write_text("not a directory", encoding="utf-8")

    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)

```

To make these tests accurate and non-brittle, you should:

1. Ensure that the remote version for `"local-plugin"` returned by `FakeClient` matches `"2.0.0"` (or adjust the equal-version test to use whatever version `FakeClient` currently exposes for `"local-plugin"`).
2. Ensure that `"3.0.0"` (or the value you choose in the “newer” test) is strictly greater than the remote version as compared by `VersionComparator.compare_version`.

If the remote plugin names or versions in `FakeClient` differ from `"local-plugin"` and `"2.0.0"`, update the plugin name/version literals in the new tests accordingly so that one case truly represents `local == remote` and the other `local > remote`.
</issue_to_address>

### Comment 3
<location path="tests/unit/test_cli_plugin_utils.py" line_range="69-78" />
<code_context>
+    assert len(plugins) == 3
+
+
+def test_build_plug_list_treats_file_plugin_path_as_empty_local_set(
+    monkeypatch, tmp_path
+):
+    plugins_file = tmp_path / "plugins"
+    plugins_file.write_text("not a directory", encoding="utf-8")
+
+    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)
+
+    plugins = build_plug_list(plugins_file)
+
+    assert [plugin["name"] for plugin in plugins] == ["local-plugin", "remote-only"]
+    assert all(plugin["status"] == PluginStatus.NOT_INSTALLED for plugin in plugins)
</code_context>
<issue_to_address>
**suggestion (testing):** Consider also testing behavior when the plugins path does not exist, not just when it is a regular file.

Since the implementation branches on `plugins_dir.exists()`, it would be helpful to add a test where the path does not exist and verify we still return only the remote plugins as `NOT_INSTALLED`. That makes the behavior explicit and guards against regressions if this logic changes later.
</issue_to_address>

### Comment 4
<location path="tests/test_cli_plugin_utils.py" line_range="1-10" />
<code_context>
+from pathlib import Path
</code_context>
<issue_to_address>
**issue (testing):** This file duplicates the tests and helpers from `tests/unit/test_cli_plugin_utils.py`; consider consolidating to a single location.

These helpers and the `test_build_plug_list_merges_local_and_remote_plugins` test are duplicated from `tests/unit/test_cli_plugin_utils.py`, which risks divergence over time. Please either keep the test suite in a single location or factor the shared helpers into a common test utility module that both suites import.
</issue_to_address>

Please address the comments from this code review:

## Overall Comments
- In `build_plug_list`, the `except Exception as e` around the HTTP call is quite broad; consider catching `httpx.RequestError` (and optionally `httpx.HTTPStatusError`) instead so that truly unexpected errors still surface.
- Some tests assert exact list ordering of plugin names (e.g., `['local-plugin', 'remote-only']`), which ties the tests to the specific iteration order of the current implementation; consider using set comparisons or order-insensitive checks to make them more resilient to internal changes.

## Individual Comments

### Comment 1
<location path="tests/unit/test_cli_plugin_utils.py" line_range="69-78" />
<code_context>
+    assert len(plugins) == 3
+
+
+def test_build_plug_list_treats_file_plugin_path_as_empty_local_set(
+    monkeypatch, tmp_path
+):
+    plugins_file = tmp_path / "plugins"
+    plugins_file.write_text("not a directory", encoding="utf-8")
+
+    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)
+
+    plugins = build_plug_list(plugins_file)
+
+    assert [plugin["name"] for plugin in plugins] == ["local-plugin", "remote-only"]
+    assert all(plugin["status"] == PluginStatus.NOT_INSTALLED for plugin in plugins)
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Avoid relying on plugin order in assertions to make the test less brittle.

Both this test and `test_build_plug_list_non_existent_path` assert the exact list `['local-plugin', 'remote-only']`. Even though the current implementation and `FakeResponse` make this order deterministic, it would be better if the tests didn’t depend on ordering. For example, you could compare the set of names or sort the names before asserting.

Suggested implementation:

```python
    assert {plugin["name"] for plugin in plugins} == {"local-plugin", "remote-only"}

```

```python
    assert {plugin["name"] for plugin in plugins} == {"local-plugin", "remote-only"}

```
</issue_to_address>

### Comment 2
<location path="tests/unit/test_cli_plugin_utils.py" line_range="83-100" />
<code_context>
+    assert all(plugin["status"] == PluginStatus.NOT_INSTALLED for plugin in plugins)
+
+
+def test_build_plug_list_local_version_equal_or_newer(monkeypatch, tmp_path):
+    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)
+
+    # 1. test if local version == remote version
+    dir_equal = tmp_path / "dir_equal"
+    write_metadata(dir_equal / "local-plugin", "local-plugin", "2.0.0")
+    
+    plugins_equal = build_plug_list(dir_equal)
+    plugins_equal_by_name = {p["name"]: p for p in plugins_equal}
+    assert plugins_equal_by_name["local-plugin"]["status"] == PluginStatus.INSTALLED
+
+    # 2. test if local version > remote version
+    dir_newer = tmp_path / "dir_newer"
+    write_metadata(dir_newer / "local-plugin", "local-plugin", "3.0.0")
+    
+    plugins_newer = build_plug_list(dir_newer)
+    plugins_newer_by_name = {p["name"]: p for p in plugins_newer}
+    assert plugins_newer_by_name["local-plugin"]["status"] == PluginStatus.INSTALLED
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for remote-only plugins in the equal/newer version scenarios.

The current test only verifies the `local-plugin` status. Please also assert that the `remote-only` plugin is present and remains `NOT_INSTALLED` in both `dir_equal` and `dir_newer` to ensure merge behavior for remote-only plugins is covered when local versions change.

```suggestion
def test_build_plug_list_local_version_equal_or_newer(monkeypatch, tmp_path):
    monkeypatch.setattr("astrbot.cli.utils.plugin.httpx.Client", FakeClient)

    # 1. test if local version == remote version
    dir_equal = tmp_path / "dir_equal"
    write_metadata(dir_equal / "local-plugin", "local-plugin", "2.0.0")

    plugins_equal = build_plug_list(dir_equal)
    plugins_equal_by_name = {p["name"]: p for p in plugins_equal}

    # local plugin should be treated as installed when versions are equal
    assert plugins_equal_by_name["local-plugin"]["status"] == PluginStatus.INSTALLED
    # remote-only plugin should still be present and not installed
    assert "remote-only" in plugins_equal_by_name
    assert plugins_equal_by_name["remote-only"]["status"] == PluginStatus.NOT_INSTALLED

    # 2. test if local version > remote version
    dir_newer = tmp_path / "dir_newer"
    write_metadata(dir_newer / "local-plugin", "local-plugin", "3.0.0")

    plugins_newer = build_plug_list(dir_newer)
    plugins_newer_by_name = {p["name"]: p for p in plugins_newer}

    # local plugin should be treated as installed when local version is newer
    assert plugins_newer_by_name["local-plugin"]["status"] == PluginStatus.INSTALLED
    # remote-only plugin should still be present and not installed
    assert "remote-only" in plugins_newer_by_name
    assert plugins_newer_by_name["remote-only"]["status"] == PluginStatus.NOT_INSTALLED
```
</issue_to_address>